Skip to content

feat(gmail): add --include-attachments to messages search - #962

Open
ronny-rentner wants to merge 6 commits into
openclaw:mainfrom
ronny-rentner:feat/gmail-include-attachments
Open

feat(gmail): add --include-attachments to messages search#962
ronny-rentner wants to merge 6 commits into
openclaw:mainfrom
ronny-rentner:feat/gmail-include-attachments

Conversation

@ronny-rentner

@ronny-rentner ronny-rentner commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds --include-attachments to gmail messages search. With it, each result carries its attachment metadata — filename, size, mimeType (and the attachmentId in JSON) — so you can see what a message holds without switching to --include-body.

Today the only way to see a message's attachments in search results is --include-body, which also decodes and renders the full body. --include-attachments gives just the attachment list.

  • New flag --include-attachments (env GOG_GMAIL_INCLUDE_ATTACHMENTS).
  • Emits the existing attachmentOutput shape (the same one --include-body already produces), so the JSON is purely additive — no existing field changes.
  • Text output gains an ATTACHMENTS column (filename (mimeType, size)); JSON gains the attachments array (with the attachmentId).
  • Fetches format=full — the same complete fetch gmail get / thread / drafts already use — so attachments at any MIME nesting depth are listed. The body data is transferred but never rendered.

Motivation

Lets a caller enumerate what a message carries — and, from the JSON, get the attachmentId to download — without switching to --include-body.

User-facing changes

  • New flag --include-attachments on gmail messages search.
  • New env var GOG_GMAIL_INCLUDE_ATTACHMENTS (mirrors the existing GOG_GMAIL_INLINE_MAX_BYTES pattern).
  • When set: an ATTACHMENTS column in text output and an additive attachments array in JSON.

Testing

  • TestExecute_GmailMessagesSearch_IncludeAttachments: lists an attachment nested several MIME levels down (proving the full fetch), does not render the body, and asserts the fetch uses format=full with no capping parts mask; the default search lists neither.
  • make ci clean (fmt / lint / deadcode / tests / docs).

Verification

Redacted terminal output from the built binary against a live Gmail account (ids masked, attachmentId truncated; sizes, mimeType, sender, subject verbatim).

Text output — the ATTACHMENTS column:

$ gog gmail messages search "from:purchase-noreply@twitch.tv has:attachment after:2026/08/01" --include-attachments
ID                THREAD            DATE              FROM                        SUBJECT          LABELS                         ATTACHMENTS
19fd72XXXXXXXXXX  19fd72XXXXXXXXXX  2026-08-06 14:41  purchase-noreply@twitch.tv  Kaufbestätigung  UNREAD,CATEGORY_UPDATES,INBOX  invoice_EU-DEXXXXXXX.pdf (application/pdf, 21.1 KB)

JSON — same metadata plus the per-fetch attachmentId:

$ gog gmail messages search "from:purchase-noreply@twitch.tv has:attachment after:2026/08/01" --include-attachments --json | jq '.messages[0].attachments'
[
  {
    "filename": "invoice_EU-DEXXXXXXX.pdf",
    "size": 21609,
    "sizeHuman": "21.1 KB",
    "mimeType": "application/pdf",
    "attachmentId": "ANGjdJ_5g7aKXXXX… (426 chars)"
  }
]

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 54c58df304

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

}

items, err := fetchMessageDetails(ctx, svc, messages, idToName, loc, c.IncludeBody, c.BodyFormat)
items, err := fetchMessageDetails(ctx, svc, messages, idToName, loc, c.IncludeBody, c.BodyFormat, c.IncludeAttachments)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Render attachments in non-JSON search output

When --include-attachments is used with the default table output or --plain, this path fetches and populates messageItem.Attachments, but the later call to gmailMessageColumns(c.IncludeBody, c.Full) never reads that field. The flag therefore performs the additional full-format API requests while producing exactly the same stdout as a search without the flag. Pass the option into the presentation layer and render the metadata, or explicitly restrict and document the flag as JSON-only.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Aug 7, 2026
@clawsweeper

clawsweeper Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed August 8, 2026, 12:04 PM ET / 16:04 UTC.

ClawSweeper review

What this changes

Adds an opt-in Gmail message-search flag and environment variable that return attachment metadata without rendering decoded message bodies.

Merge readiness

⚠️ Needs maintainer review before merge - 3 items remain

Keep open for a maintainer product-direction decision. The feature has sufficient redacted live CLI proof, but attachment filenames and MIME values are written unescaped into the table, so a tab or line break can corrupt text/--plain output.

Likely related people: Peter Steinberger (high confidence) and chrischall (medium confidence) from the Gmail search and attachment-output history.

Priority: P2
Reviewed head: 64754e141172ba59d8597fa950bc56e5c0c188f3
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Live CLI evidence and focused coverage are useful, but the table-output control-character defect must be corrected before merge.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (terminal): The PR body contains redacted terminal output from a live Gmail account showing the new text column and JSON attachment metadata after the change.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The PR body contains redacted terminal output from a live Gmail account showing the new text column and JSON attachment metadata after the change.
Evidence reviewed 5 items Current main and the current release lack the feature: Current main and v0.34.2 still call the existing detail fetch and table presentation with only the body option; neither contains the proposed attachment-search flag.
The patch implements deep attachment discovery without rendering body output: The proposed fetch switches to full Gmail message format when attachments are requested, then collects attachments while leaving Body unset unless the separate body flag is enabled.
Table rendering can be malformed by attachment metadata: The new ATTACHMENTS column interpolates raw Gmail filename and MIME values, while the table writer joins columns with tabs and appends a newline without escaping values.
Findings 1 actionable finding [P2] Sanitize attachment metadata before table rendering
Security None None.

How this fits together

Gmail message search lists matching IDs, fetches message details when needed, and renders JSON or a text table. This option extends the detail-fetch and presentation stages to expose attachment metadata.

flowchart LR
  A[Gmail search query] --> B[Matching message IDs]
  B --> C[Message detail fetch]
  C --> D[Attachment metadata collection]
  D --> E[JSON result]
  D --> F[Text or plain table]
Loading

Decision needed

Question Recommendation
Should gogcli add an opt-in gmail messages search --include-attachments flag and GOG_GMAIL_INCLUDE_ATTACHMENTS environment setting? Sponsor the focused search feature: Accept the opt-in attachment-listing workflow, after correcting text-table escaping and retaining the existing JSON attachment shape.

Why: Current main and the latest release do not establish this command or environment-variable contract, so acceptance is a product decision rather than a repair of documented behavior.

Before merge

  • Sanitize attachment metadata before table rendering (P2) - --include-attachments sends Gmail-controlled filenames and MIME values directly to WriteTable, which uses tabs and newlines as table structure. A filename containing either character adds a column or row in default and --plain output; replace those control characters before composing this cell and cover that case.
  • Complete next step (P2) - A maintainer must sponsor or decline the new command and environment-variable contract before the narrow escaping repair is pursued.

Findings

  • [P2] Sanitize attachment metadata before table rendering — internal/cmd/gmail_presentation.go:55
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope 167 added, 18 removed, 6 files changed The change is contained to Gmail search, shared presentation, tests, and generated command documentation.

Technical review

Best possible solution:

If a maintainer sponsors this opt-in CLI surface, preserve the shared JSON attachment shape and sanitize attachment text metadata before it enters the human or plain table renderer.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: a Gmail attachment filename or MIME value containing a tab, carriage return, or newline reaches the new table cell unchanged, while the table writer uses those characters as output structure.

Is this the best way to solve the issue?

No: the feature design needs explicit maintainer sponsorship, and the new table cell must escape control characters to preserve parseable text output.

Full review comments:

  • [P2] Sanitize attachment metadata before table rendering — internal/cmd/gmail_presentation.go:55
    --include-attachments sends Gmail-controlled filenames and MIME values directly to WriteTable, which uses tabs and newlines as table structure. A filename containing either character adds a column or row in default and --plain output; replace those control characters before composing this cell and cover that case.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8fe3e7995d0b.

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body contains redacted terminal output from a live Gmail account showing the new text column and JSON attachment metadata after the change.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp. Replaced prior rating: 🧂 unranked krab.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.
  • remove merge-risk: 🚨 security-boundary: Current PR review selected no merge-risk labels.

Label justifications:

  • P2: This is a bounded optional Gmail CLI enhancement with one normal-priority output-correctness repair.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp. Replaced prior rating: 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Sufficient (terminal): The PR body contains redacted terminal output from a live Gmail account showing the new text column and JSON attachment metadata after the change.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body contains redacted terminal output from a live Gmail account showing the new text column and JSON attachment metadata after the change.

Evidence

What I checked:

  • Current main and the current release lack the feature: Current main and v0.34.2 still call the existing detail fetch and table presentation with only the body option; neither contains the proposed attachment-search flag. (internal/cmd/gmail_messages.go:29, 8fe3e7995d0b)
  • The patch implements deep attachment discovery without rendering body output: The proposed fetch switches to full Gmail message format when attachments are requested, then collects attachments while leaving Body unset unless the separate body flag is enabled. (internal/cmd/gmail_messages.go:248, 64754e141172)
  • Table rendering can be malformed by attachment metadata: The new ATTACHMENTS column interpolates raw Gmail filename and MIME values, while the table writer joins columns with tabs and appends a newline without escaping values. (internal/cmd/gmail_presentation.go:55, 64754e141172)
  • Focused command coverage and live proof are present: The added HTTP-level command test verifies a deeply nested attachment, no rendered body, full-format fetching, and unchanged default output; the PR body also supplies redacted live text and JSON terminal output. (internal/cmd/execute_gmail_messages_include_attachments_test.go:77, 64754e141172)
  • Feature-history routing: History shows Peter Steinberger as the dominant Gmail search and attachment-output contributor, with chrischall recently changing message-listing output in commit 3c9466a. (internal/cmd/gmail_messages.go:194, 3c9466af5171)

Likely related people:

  • Peter Steinberger: History attributes the foundational search presentation and centralized attachment-output work to Peter Steinberger. (role: Gmail search and attachment-output contributor; confidence: high; commits: 5095d7cb34ff, 0904cdb2c327; files: internal/cmd/gmail_messages.go, internal/cmd/gmail_attachments.go)
  • chrischall: Commit 3c9466a recently updated the message-listing output model. (role: recent message-listing contributor; confidence: medium; commits: 3c9466af5171; files: internal/cmd/gmail_messages.go)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Escape tab, carriage-return, and newline characters in attachment values used by the text/plain table and add a regression test.
  • Obtain maintainer confirmation that this new Gmail search flag and environment-variable surface should ship.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (12 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-07T19:27:02.648Z sha d57234f :: needs real behavior proof before merge. :: [P2] Render attachment identifiers and MIME types in text output | [P2] Avoid silently truncating attachment discovery at depth eight
  • reviewed 2026-08-07T22:01:46.611Z sha d57234f :: needs real behavior proof before merge. :: [P2] Do not silently omit deeply nested attachments | [P2] Expose MIME type and identifier in text output
  • reviewed 2026-08-08T03:01:07.784Z sha d57234f :: needs real behavior proof before merge. :: [P2] Avoid silently omitting deeply nested attachments | [P2] Render MIME type and identifier in text output
  • reviewed 2026-08-08T04:40:59.083Z sha d57234f :: needs real behavior proof before merge. :: [P2] Fetch attachment metadata at every MIME depth | [P2] Expose the advertised metadata in text output
  • reviewed 2026-08-08T04:48:33.173Z sha 14ae959 :: needs real behavior proof before merge. :: [P1] Avoid fetching raw bodies for attachment-only searches
  • reviewed 2026-08-08T05:26:25.627Z sha eb83a8b :: needs real behavior proof before merge. :: [P1] Avoid fetching raw bodies for attachment-only searches | [P2] Emit download-capable IDs in text output
  • reviewed 2026-08-08T07:38:14.007Z sha d92c681 :: needs real behavior proof before merge. :: [P1] Keep attachment-only searches from downloading body data | [P2] Print complete attachment IDs in text output
  • reviewed 2026-08-08T09:11:39.825Z sha 64754e1 :: needs real behavior proof before merge. :: [P1] Avoid fetching body data for attachment-only searches | [P2] Keep download-capable attachment IDs in text output

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Aug 7, 2026
@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. and removed merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Aug 7, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. label Aug 8, 2026
@ronny-rentner

ronny-rentner commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

On the "render MIME type and identifier in text output" finding: mimeType is now in the ATTACHMENTS column; the attachmentId deliberately is not. Two reasons:

1. The full id doesn't fit a table column. Gmail's attachmentId is ~426 bytes. Dropping that into a cell blows the table apart — it's unusable next to filename, mimeType, and size.

2. A short id isn't feasible either. gogcli already uses a short (first-8-char) form of the attachmentId to tag saved filenames, so the obvious idea was to show that same short id in the column. But the attachmentId is a per-request opaque token — Gmail mints a fresh ~426-char value on every messages.get, and all of them stay valid to download by. Verified against a live account, three fetches of the same message returned three different ids (leading bytes shown, the rest masked):

ANGjdJ-pXXXX…
ANGjdJ9oXXXX…
ANGjdJ9vXXXX…

and an id captured in one call still downloaded fine in a separate later call. Because the id changes on every call, its short form changes on every search too — it would be neither stable nor a match for a later download's filename, so it conveys nothing a caller can act on.

So the column shows filename (mimeType, size). The full per-fetch id stays in --json (valid for that fetch), which is where retrieval belongs; the stable, compact reference is the 0-based index in the stacked #963.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant